Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

twinkle: Update vector classes (vectorMenu -> vector-menu) for the Twinkle menu #962

Merged
merged 1 commit into from
Jun 17, 2020

Conversation

Amorymeltzer
Copy link
Collaborator

@Amorymeltzer Amorymeltzer commented Jun 8, 2020

The work is a little spread out, but see phabricator.wikimedia.org/T253329, phabricator.wikimedia.org/T254798, and phabricator.wikimedia.org/T254797.

Basically, for our purposes:

  • vectorMenu replaced with vector-menu
    • Dropdown also gets vector-menu-dropdown
      • twinkle.css updated to be specific so that tabs doesn't break (it is now?)
    • Tabs also gets vector-menu-tabs
  • vectorMenuCheckbox replaced with vector-menu-checkbox
  • Add an innerDiv with vector-menu-content
  • menu replaced with vector-menu-content-list, although this isn't unique

A lot of this is in flux at the moment (e.g. emptyPortlet -> vector-menu-empty). The Vector skin is keeping all the old classes for now, but I'd like not to create (even) more work.

@Amorymeltzer Amorymeltzer added the Module: twinkle The twinkle.js global gadget file label Jun 8, 2020
@Amorymeltzer Amorymeltzer added this to the June 2020 update milestone Jun 8, 2020
@Amorymeltzer
Copy link
Collaborator Author

AFAICT vectorTabs can be replaced with vector-menu-tabs as well, but that's not changing right now. Plus, it's a mess and I'm not sure what the final state is supposed to be; right now, it looks like they're vector-menu vectorTabs vector-menu-tabs.

Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Jun 9, 2020
This is all probably unnecessary, but change to match new Vector: https://phabricator.wikimedia.org/T66477 specifically https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/594819 (although (temporarily) reverted in https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/595627 and restored in https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/596312).  I've just gone and removed the `role`.

Of course, `outerDiv` is now poorly named.  Worth renaming all of this, or just busy work?  See also wikimedia-gadgets#962 for changes in the menu classes.
@Amorymeltzer Amorymeltzer removed this from the June 2020 update milestone Jun 12, 2020
@Amorymeltzer
Copy link
Collaborator Author

*heavy sigh* There are like a half dozen things that are in flux right now, and AFAICT the wikis themselves are not ready. I'd like to avoid a lot of future maintenance, and so I've got some additional work for future-proofing, but I think most of it will have to wait. I think the barebones will be functional once again this week so I can test it more easily on testwiki after tomorrow.

@Amorymeltzer Amorymeltzer force-pushed the vectormenu branch 2 times, most recently from 1adda85 to d934c1f Compare June 16, 2020 10:45
@Amorymeltzer Amorymeltzer changed the title twinkle: Update vector classes (vectorMenu -> vector-menu) twinkle: Update vector classes (vectorMenu -> vector-menu) for the Twinkle menu Jun 16, 2020
@Amorymeltzer Amorymeltzer marked this pull request as ready for review June 16, 2020 10:52
@Amorymeltzer
Copy link
Collaborator Author

Amorymeltzer commented Jun 16, 2020

Okay @Xi-Plus @MusikAnimal I've pushed and tested this on testwiki, as far as I can tell everything looks good! As above, things are in a weird place right now, so DO let me know if you can find any issues. I reworked things a bit (last modified by @siddharthvp in #763) but looks like I fixed a bug or two. Tested with vector/modern/monobook/timeless.

@Amorymeltzer
Copy link
Collaborator Author

I think this (and #963) can be merged and pushed live, so if I hear no complains I'll do that tonight. @Xi-Plus, would it be easier for you if I just merged these/this now?

Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Jun 16, 2020
This is all probably unnecessary, but change to match new Vector: https://phabricator.wikimedia.org/T66477 specifically https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/594819 (although (temporarily) reverted in https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/595627 and restored in https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/596312).  I've just gone and removed the `role`.

Of course, `outerDiv` is now poorly named.  Worth renaming all of this, or just busy work?  See also wikimedia-gadgets#962 for changes in the menu classes.  This applies the change to all skins, but MW is only changing Vector.  That's probably fine.
@Xi-Plus
Copy link
Contributor

Xi-Plus commented Jun 16, 2020

When should I deploy the patch to the site?

…inkle menu

The work is a little spread out, but see https://phabricator.wikimedia.org/T253329, https://phabricator.wikimedia.org/T254798, and https://phabricator.wikimedia.org/T254797.

Basically, for our purposes:

- `vectorMenu` replaced with `vector-menu`
    - Dropdown also gets `vector-menu-dropdown`
        - twinkle.css updated to be specific so that tabs doesn't break (it is now?)
    - Tabs also gets `vector-menu-tabs`
- `vectorMenuCheckbox` replaced with `vector-menu-checkbox`
- Add an innerDiv with `vector-menu-content`
- `menu` replaced with `vector-menu-content-list`, although this isn't unique

A lot of this is in flux at the moment (e.g. `emptyPortlet` -> `vector-menu-empty`).  The Vector skin is keeping all the old classes for now, but I'd like not to create (even) more work.  `menu` and `vectorMenuCheckbox` are kept temporarily to ease testing and to allow for smooth updating before 1.35-wmf.37 (https://www.mediawiki.org/wiki/MediaWiki_1.35/wmf.37) goes live, since not all classes are present currently.  Note, this means the Twinkle menu won't stay open with a click until that update, since that contained the fix for the p-cactions menu itself (see https://phabricator.wikimedia.org/T254851 and https://phabricator.wikimedia.org/T255069, among others).
@Amorymeltzer
Copy link
Collaborator Author

Amorymeltzer commented Jun 16, 2020

@Xi-Plus I think right away. When testwiki got updated (last night EDT) to the latest (1.35-wmf.37), that messed up the Vector menus, so it should be right now. Wikipedias will be updated Thursday, so unless there's a major change, I'd imagine it'd need to be out by then.

I just did a quick test, and it looks like not all of the updated classes are present in wmf.36, so I had to re-add menu and vectorMenuCheckbox for safety. With some quick testing during naptime, I think this latest version (8f7b2f3) should be safe to put up any time starting now.

As noted in the latest commit, it means the dropdown won't work properly until wmf.37, so you could wait a bit if you like. This whole roll out has not been well timed/planned IMO.

@Amorymeltzer
Copy link
Collaborator Author

I'll plan on merging this tonight (EDT) and pushing live to enwiki ahead of the update later this week.

Copy link
Collaborator

@MusikAnimal MusikAnimal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, and is consistent with what I had to do for MoreMenu. I summarized the changes that we both and probably others will have to make at https://phabricator.wikimedia.org/T254797#6230055

@Amorymeltzer Amorymeltzer merged commit 17d664f into wikimedia-gadgets:master Jun 17, 2020
@Amorymeltzer Amorymeltzer deleted the vectormenu branch June 17, 2020 00:31
@Amorymeltzer
Copy link
Collaborator Author

Thanks for the 👍 @MusikAnimal — your comments there cover it pretty well!

I do worry that the disparate Twinkle installations are going to run into some serious problems, especially the older ones, so hopefully that's a good roadmap. I'll post at en's WT:TW in case anyone ends up there.

Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Jun 20, 2020
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Jul 2, 2020
Amorymeltzer added a commit that referenced this pull request Jul 9, 2020
twinkle: Remove leftover classes from #962, rename outerdiv to outernav
siddharthvp pushed a commit to siddharthvp/twinkle that referenced this pull request Jul 10, 2020
wiki-ST47 pushed a commit to wiki-ST47/twinkle that referenced this pull request Sep 2, 2020
This is all probably unnecessary, but change to match new Vector: https://phabricator.wikimedia.org/T66477 specifically https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/594819 (although (temporarily) reverted in https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/595627 and restored in https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/596312).  I've just gone and removed the `role`.

Of course, `outerDiv` is now poorly named.  Worth renaming all of this, or just busy work?  See also wikimedia-gadgets#962 for changes in the menu classes.  This applies the change to all skins, but MW is only changing Vector.  That's probably fine.
wiki-ST47 pushed a commit to wiki-ST47/twinkle that referenced this pull request Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Module: twinkle The twinkle.js global gadget file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The selectors .menu and .vectorMenu will no longer work in the Vector skin
3 participants